Conversation
nirinchev
left a comment
There was a problem hiding this comment.
Some wording comments + code snippet suggestions. None of these are "must fix" or anything, so if you don't agree, feel free to disregard.
| public async System.Threading.Tasks.Task LogsOnManyWays() | ||
| { | ||
| // :code-block-start: logon_anon | ||
| Realms.Sync.User anonUser = await app.LogInAsync(Credentials.Anonymous()); |
There was a problem hiding this comment.
Not sure if that's the style of the docs, but I feel that using the fully qualified name is overly verbose and doesn't convey much useful information. It's also not very copy-pasteable as in my experience most projects use vars for variables and a few use the type name (without the namespace).
There was a problem hiding this comment.
@nirinchev - the reason I had the FQDN here was because we have a user-defined class called "User", so the compiler complained. I've updated the custom class names because they are semi-arbitrary.
There was a problem hiding this comment.
My suggestion would be to always use var as the variable name should be indicative enough. E.g. anonUser is obviously a User, so explicitly specifying the type is redundant. This also lets you avoid some of the type collisions.
source/dotnet/authenticate.txt
Outdated
| .. _dotnet-login: | ||
| {+service+} provides an API for authenticating users using any enabled | ||
| authentication provider. Instantiate a ``Credentials`` object and pass | ||
| it to the ``app.login()`` method to authenticate a user and create a ``User`` |
There was a problem hiding this comment.
The .NET method is LogInAsync - I don't think anyone would get confused over it, but it's always better to use the exact method names.
"to authenticate a user and create a User object" sounds a bit repetitive. Additionally, the "create a User" wording is a bit imprecise as it implies constructing the instance as opposed to obtaining a reference to it by calling a method. Don't have suggestions for avoiding the repetition, but "create a User object" can probably be reworded as "obtain a User instance".
source/dotnet/authenticate.txt
Outdated
| object. Each authentication provider corresponds to a method used to | ||
| instantiate ``Credentials`` objects using that authentication provider. |
There was a problem hiding this comment.
This sounds a bit heavy - perhaps if we inverted the flow, it would be easier to follow. I.e. something like "The Credentials class exposes factory methods for each authentication provider."
| - Credentials Generation Method | ||
|
|
||
| * - :ref:`Anonymous <dotnet-login-anonymous>` | ||
| - ``Credentials.Anonymous()`` |
There was a problem hiding this comment.
Would be nice if those linked to the API docs for that method as those typically contain valuable information about parameters, return types, etc.
I can send you a .zip containing the API docs for the v10 SDK if that would help.
There was a problem hiding this comment.
Yes, please...or a link would be fine, too.
| var functionParameters = new Dictionary<string, string>() | ||
| { | ||
| { "username", "caleb" }, | ||
| { "password", "shhhItsASektrit!" }, | ||
| { "someOtherProperty", "cheesecake" } | ||
| }; | ||
| Realms.Sync.User functionUser = | ||
| await app.LogInAsync(Credentials.Function(functionParameters)); |
There was a problem hiding this comment.
This will work, but the code can be simplified by using anonymous objects:
var functionParameters = new
{
username = "caleb",
password = "shhhItsASektrit",
someOtherProperty = "cheesecake"
};
var functionUser = await app.LogInAsync(Credentials.Function(functionParameters));Note that when using the anonymous object syntax, you can also hint that properties don't have to be strings only. Any serializable .NET type, including embedded objects will work:
var functionParameters = new
{
biometricsValidated = false,
todaysDate = DateTime.UtcNow
}Of course, don't have to go overboard with the extra properties, just FYI.
| { | ||
| // :code-block-start: logon_google | ||
| Realms.Sync.User googleUser = | ||
| await app.LogInAsync(Credentials.Google("<google_token>")); |
There was a problem hiding this comment.
I think we're calling this google auth code now as using the access token flow doesn't work.
| } | ||
| catch (Exception e) | ||
| { | ||
| Assert.AreEqual("http error code considered fatal: Client Error: 401", e.Message); |
There was a problem hiding this comment.
FYI: this message will change when you upgrade to the latest v10 package - the generic wording was due to a bug that has now been addressed and the message should be more meaningful (I don't think that's exposed in the docs, but will break your test, so don't be surprised).
| // :code-block-end: | ||
| user = app.LogInAsync(Credentials.EmailPassword("foo@foo.com", "foobar")).Result; | ||
| config = new SyncConfiguration("My Project", user); | ||
| Realm realm = await Realm.GetInstanceAsync(config); |
There was a problem hiding this comment.
Probably move realm to a class-level variable as well to avoid the Realm realm = await Realm... repetition.
There was a problem hiding this comment.
Since these code snippets live "alone" within the docs, and the user won't be seeing this unit test file as a whole, I think it's useful to have the Realm there to be clear what we are building.
There was a problem hiding this comment.
Right, but it's in the variable name as well - i.e. knowing that I have a variable called realm of type Realm isn't that much more useful than just knowing that I have a variable called realm.
examples/dotnet/Examples/Examples.cs
Outdated
| config = new SyncConfiguration("My Project", user); | ||
| Realm realm = await Realm.GetInstanceAsync(config); | ||
| // :code-block-stat: create | ||
| RealmTask testTask = new RealmTask() |
There was a problem hiding this comment.
Same here - RealmTask testTask = new RealmTask is fairly repetitive. Also, you can drop the () from the constructor when using the property initializer syntax - there are different opinions about it, but most high profile .NET people seem to omit them.
examples/dotnet/Examples/Examples.cs
Outdated
| Realm realm = await Realm.GetInstanceAsync(config); | ||
| // :code-block-end: | ||
| // :code-block-start: read-all | ||
| var tasks = realm.All<RealmTask>().ToList(); |
There was a problem hiding this comment.
We shouldn't encourage people to .ToList() query results as that removes the "liveness" from the collection.
examples/dotnet/Examples/Examples.cs
Outdated
| // :code-block-end: | ||
| Assert.AreEqual(1, tasks.Count); | ||
| // :code-block-start: read-some | ||
| tasks = realm.All<RealmTask>().Where(t => t.Status == "Open").ToList(); |
There was a problem hiding this comment.
Same here - materializing the collection is generally viewed as a bad practice unless you have very specific needs. Instead, you should operate on the query as much as possible.
| { | ||
| username= "caleb", | ||
| password = "shhhItsASektrit!", | ||
| IQ = 42, |
There was a problem hiding this comment.
Hahaha... I guess explains the password 😅
There was a problem hiding this comment.
Can you tell I was getting punchy?
ccho-mongodb
left a comment
There was a problem hiding this comment.
Strange, my review seems to never have gone through. Sorry about that.
I'm certain that I typed a message, selected Approve, and submitted, but I don't see that message nor the approval.
Anyway, hope these are helpful.
| await functionUser.LogOutAsync(); | ||
| // :code-block-start: logon_JWT | ||
| Realms.Sync.User jwtUser = | ||
| await app.LogInAsync(Credentials.JWT("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkNhbGViIiwiaWF0IjoxNjAxNjc4ODcyLCJleHAiOjI1MTYyMzkwMjIsImF1ZCI6InR1dHMtdGlqeWEifQ.LHbeSI2FDWrlUVOBxe-rasuFiW-etv2Gu5e3eAa6Y6k")); |
There was a problem hiding this comment.
Would it be better to call it <JWT_token> similar to the fb and google examples? And is this a real credential that should be removed prior to commit?
|
|
||
| API Key | ||
| ~~~~~~~ | ||
| If you have enabled :ref:`API Key authentication <api-key-authentication>`, |
There was a problem hiding this comment.
Since you've added Authentication to Anonymous and Email/Password headers, should you also update the rest of the authn method headers?
|
|
||
| Custom Function | ||
| ~~~~~~~~~~~~~~~ | ||
| If you have enabled the |
There was a problem hiding this comment.
Nitpick: I think we prefer past tense over past perfect. E.g. "If you enabled ..." instead of "If you have enabled..." Applies to all instances.
nirinchev
left a comment
There was a problem hiding this comment.
Final nitpick from me.
| public async System.Threading.Tasks.Task LogsOnManyWays() | ||
| { | ||
| // :code-block-start: logon_anon | ||
| User anonUser = await app.LogInAsync(Credentials.Anonymous()); |
There was a problem hiding this comment.
I don't think we should prefix the variable name with the provider type - in the iOS/Android docs, we're using user for all the examples. Since that will result in compilation errors here, you can add a scope around each snippet:
public async System.Threading.Tasks.Task LogsOnManyWays()
{
{
// :code-block-start: logon_anon
var user = await app.LogInAsync(CredentialAnonymous());
// :code-block-end:
Assert.AreEqual(UserState.LoggedIn, user.State);
await user.LogOutAsync();
}
{
// :code-block-start: logon_EP
var user = await app.LogInAsync(
Credentials.EmailPassword("caleb@mongodb.com", "shhhItsASektrit!"));
// :code-block-end:
Assert.AreEqual(UserState.LoggedIn, user.State);
await user.LogOutAsync();
}
// ...
}<h3>Snyk has created this PR to upgrade bson from 4.7.0 to 4.7.1.</h3>
:information_source: Keep your dependencies up-to-date. This makes it
easier to fix existing vulnerabilities and to more quickly identify and
fix newly disclosed vulnerabilities when they affect your project.
<hr/>
- The recommended version is **1 version** ahead of your current
version.
- The recommended version was released **21 days ago**, on 2023-01-05.
<details>
<summary><b>Release notes</b></summary>
<br/>
<details>
<summary>Package name: <b>bson</b></summary>
<ul>
<li>
<b>4.7.1</b> - <a
href="https://snyk.io/redirect/github/mongodb/js-bson/releases/tag/v4.7.1">2023-01-05</a></br><p>The
MongoDB Node.js team is pleased to announce version v4.7.1 of the bson
package!</p>
<h3>Bug Fixes</h3>
<ul>
<li><strong><a class="issue-link js-issue-link notranslate"
rel="noopener noreferrer nofollow"
href="https://jira.mongodb.org/browse/NODE-4905">NODE-4905</a>:</strong>
double precision accuracy in canonical EJSON (<a
href="https://snyk.io/redirect/github/mongodb/js-bson/issues/549"
data-hovercard-type="pull_request"
data-hovercard-url="/mongodb/js-bson/pull/549/hovercard">#549</a>) (<a
href="https://snyk.io/redirect/github/mongodb/js-bson/commit/d86bd52661e7f5d26479f6b63acac7950f505d69">d86bd52</a>)</li>
</ul>
<h2>Documentation</h2>
<ul>
<li>API: <a
href="https://snyk.io/redirect/github/mongodb/js-bson#readme">https://github.com/mongodb/js-bson#readme</a></li>
<li>Changelog: <a
href="https://snyk.io/redirect/github/mongodb/js-bson/blob/4.0/HISTORY.md#change-log">https://github.com/mongodb/js-bson/blob/4.0/HISTORY.md#change-log</a></li>
</ul>
<p>We invite you to try the bson library immediately, and report any
issues to the <a href="https://jira.mongodb.org/projects/NODE"
rel="nofollow">NODE project</a>.</p>
</li>
<li>
<b>4.7.0</b> - <a
href="https://snyk.io/redirect/github/mongodb/js-bson/releases/tag/v4.7.0">2022-08-18</a></br><p>The
MongoDB Node.js team is pleased to announce version 4.7.0 of the bson
package!</p>
<h2>Release Highlights</h2>
<p>This release adds <em>automatic</em> UUID support. Now when
serializing or deserializing BSON you can work directly with the UUID
type without explicit conversion methods. The UUID class is now a
subclass of binary so all existing code will continue to work (including
the explicit conversion methods
<code>.toUUID</code>/<code>.toBinary</code>). The same automatic support
for UUID is also present in EJSON
<code>.parse</code>/<code>.stringify</code>.</p>
<p>Take a look at the following for the expected behavior:</p>
<div class="highlight highlight-source-ts notranslate position-relative
overflow-auto" data-snippet-clipboard-copy-content="const document =
BSON.deserialize(bytes)
// { uuid: UUID('xxx') }
BSON.serialize(document)
// Buffer < document with uuid (binary subtype 4) >"><pre><span
class="pl-k">const</span> <span class="pl-smi">document</span> <span
class="pl-c1">=</span> <span class="pl-smi">BSON</span><span
class="pl-kos">.</span><span class="pl-en">deserialize</span><span
class="pl-kos">(</span><span class="pl-s1">bytes</span><span
class="pl-kos">)</span>
<span class="pl-c">// { uuid: UUID('xxx') }</span>
<span class="pl-smi">BSON</span><span class="pl-kos">.</span><span
class="pl-en">serialize</span><span class="pl-kos">(</span><span
class="pl-smi">document</span><span class="pl-kos">)</span>
<span class="pl-c">// Buffer < document with uuid (binary subtype 4)
></span></pre></div>
<p>Special thanks to <a class="user-mention notranslate"
data-hovercard-type="user"
data-hovercard-url="/users/aditi-khare-mongoDB/hovercard"
data-octo-click="hovercard-link-click"
data-octo-dimensions="link_type:self"
href="https://snyk.io/redirect/github/aditi-khare-mongoDB">@
aditi-khare-mongoDB</a> for all her hard work on this feature!! <g-emoji
class="g-emoji" alias="tada"
fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/1f389.png">🎉</g-emoji></p>
<h3>Features</h3>
<ul>
<li><strong><a class="issue-link js-issue-link notranslate"
rel="noopener noreferrer nofollow"
href="https://jira.mongodb.org/browse/NODE-4405">NODE-4405</a>:</strong>
support serializing UUID class (<a
href="https://snyk.io/redirect/github/mongodb/js-bson/issues/508"
data-hovercard-type="pull_request"
data-hovercard-url="/mongodb/js-bson/pull/508/hovercard">#508</a>) (<a
href="https://snyk.io/redirect/github/mongodb/js-bson/commit/f5dc9edf915cc119f02f53ec84d1c640695dced7">f5dc9ed</a>)</li>
<li><strong><a class="issue-link js-issue-link notranslate"
rel="noopener noreferrer nofollow"
href="https://jira.mongodb.org/browse/NODE-4419">NODE-4419</a>:</strong>
UUID class deserialization (<a
href="https://snyk.io/redirect/github/mongodb/js-bson/issues/509"
data-hovercard-type="pull_request"
data-hovercard-url="/mongodb/js-bson/pull/509/hovercard">#509</a>) (<a
href="https://snyk.io/redirect/github/mongodb/js-bson/commit/ff2b97585848730fcf90cd21c14ba2a18a0ed016">ff2b975</a>)</li>
<li><strong><a class="issue-link js-issue-link notranslate"
rel="noopener noreferrer nofollow"
href="https://jira.mongodb.org/browse/NODE-4506">NODE-4506</a>:</strong>
Make UUID a subclass of binary (<a
href="https://snyk.io/redirect/github/mongodb/js-bson/issues/512"
data-hovercard-type="pull_request"
data-hovercard-url="/mongodb/js-bson/pull/512/hovercard">#512</a>) (<a
href="https://snyk.io/redirect/github/mongodb/js-bson/commit/e9afa9dcfc295da8ff53b28658835fc76cde557c">e9afa9d</a>)</li>
<li><strong><a class="issue-link js-issue-link notranslate"
rel="noopener noreferrer nofollow"
href="https://jira.mongodb.org/browse/NODE-4535">NODE-4535</a>:</strong>
automatically promote UUIDs when deserializing and parsing UUIDs (<a
href="https://snyk.io/redirect/github/mongodb/js-bson/issues/513"
data-hovercard-type="pull_request"
data-hovercard-url="/mongodb/js-bson/pull/513/hovercard">#513</a>) (<a
href="https://snyk.io/redirect/github/mongodb/js-bson/commit/1dc7eaea6a61924be66ae5b8a05b74d5dd9c7b1e">1dc7eae</a>)</li>
</ul>
<hr>
<h2>Documentation</h2>
<ul>
<li>API: <a
href="https://snyk.io/redirect/github/mongodb/js-bson#readme">https://github.com/mongodb/js-bson#readme</a></li>
<li>Changelog: <a
href="https://snyk.io/redirect/github/mongodb/js-bson/blob/main/HISTORY.md#change-log">https://github.com/mongodb/js-bson/blob/main/HISTORY.md#change-log</a></li>
</ul>
<p>We invite you to try the bson library immediately, and report any
issues to the <a href="https://jira.mongodb.org/projects/NODE"
rel="nofollow">NODE project</a>.</p>
</li>
</ul>
from <a
href="https://snyk.io/redirect/github/mongodb/js-bson/releases">bson
GitHub release notes</a>
</details>
</details>
<details>
<summary><b>Commit messages</b></summary>
</br>
<details>
<summary>Package name: <b>bson</b></summary>
<ul>
<li><a
href="https://snyk.io/redirect/github/mongodb/js-bson/commit/5465c33b356ceaed05c1759007acdf3ab077ee33">5465c33</a>
chore(release): 4.7.1</li>
<li><a
href="https://snyk.io/redirect/github/mongodb/js-bson/commit/d86bd52661e7f5d26479f6b63acac7950f505d69">d86bd52</a>
fix(NODE-4905): double precision accuracy in canonical EJSON (#549)</li>
</ul>
<a
href="https://snyk.io/redirect/github/mongodb/js-bson/compare/853bbb0441b0e29e5277cd191b515d5a884d8d21...5465c33b356ceaed05c1759007acdf3ab077ee33">Compare</a>
</details>
</details>
<hr/>
**Note:** *You are seeing this because you or someone else with access
to this repository has authorized Snyk to open upgrade PRs.*
For more information: <img
src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiIzOTdmMzA0MS1kMTJmLTQ4MDMtODIyNC1iNDY4MmQ0YzU4NjgiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6IjM5N2YzMDQxLWQxMmYtNDgwMy04MjI0LWI0NjgyZDRjNTg2OCJ9fQ=="
width="0" height="0"/>
🧐 [View latest project
report](https://app.snyk.io/org/sandbox-2ba/project/852e6e4f-be96-45c8-b370-1060f5ebee55?utm_source=github&utm_medium=referral&page=upgrade-pr)
🛠 [Adjust upgrade PR
settings](https://app.snyk.io/org/sandbox-2ba/project/852e6e4f-be96-45c8-b370-1060f5ebee55/settings/integration?utm_source=github&utm_medium=referral&page=upgrade-pr)
🔕 [Ignore this dependency or unsubscribe from future upgrade
PRs](https://app.snyk.io/org/sandbox-2ba/project/852e6e4f-be96-45c8-b370-1060f5ebee55/settings/integration?pkg=bson&utm_source=github&utm_medium=referral&page=upgrade-pr#auto-dep-upgrades)
<!---
(snyk:metadata:{"prId":"397f3041-d12f-4803-8224-b4682d4c5868","prPublicId":"397f3041-d12f-4803-8224-b4682d4c5868","dependencies":[{"name":"bson","from":"4.7.0","to":"4.7.1"}],"packageManager":"npm","type":"auto","projectUrl":"https://app.snyk.io/org/sandbox-2ba/project/852e6e4f-be96-45c8-b370-1060f5ebee55?utm_source=github&utm_medium=referral&page=upgrade-pr","projectPublicId":"852e6e4f-be96-45c8-b370-1060f5ebee55","env":"prod","prType":"upgrade","vulns":[],"issuesToFix":[],"upgrade":[],"upgradeInfo":{"versionsDiff":1,"publishedDate":"2023-01-05T15:16:00.352Z"},"templateVariants":[],"hasFixes":false,"isMajorUpgrade":false,"isBreakingChange":false,"priorityScoreList":[]})
--->
---------
Co-authored-by: snyk-bot <snyk-bot@snyk.io>
Pull Request Info
Issue JIRA link:
https://jira.mongodb.org/browse/DOCSP-12591
Docs staging link (requires sign-in on MongoDB Corp SSO):
https://docs-mongodbcom-staging.corp.mongodb.com/realm/docsworker-xlarge/12591/dotnet/authenticate.html